Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protobuffer upgrade to match goflow2 v2.2.1 #86

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ynHuber
Copy link
Collaborator

@ynHuber ynHuber commented Nov 22, 2024

Goflow2 changed its protobuffer format
These changes adapt the new format and introduce a legacy export flag to still support existing applications

Goflow2 changed its protobuffer format
These changes adapt the new format
goflow2 change the format of kafka messages. To ensure the functionallity of existing 3rd party software a legacy flag was added
flowmessages didnt contain start/end timestamps
Copy link
Collaborator

@debugloop debugloop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than my two comments, looking good 👍🏻

@@ -0,0 +1,502 @@
// FROM https://github.com/bwNetFlow/flowfilter/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But... why? This requires the flowpipeline project to have knowledge about the details of flowfilter's matching. That kinda goes against separation of concerns.

//go:build linux
// +build linux

// based on https://github.com/bwNetFlow/bpf_flowexport/blob/master/packetdump/packetdump.go
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here I guess? The reason we initially kept this outside the repo was that it stays separate and we can leverage any other kind of bpf packet capture. At this point, there should be lots of projects out there that we could use instead of our home-baked solution (I found go-pkt just now for instance)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants